feat: 1/7 expose PostgreSQL role parser surface#25521
Open
tabVersion wants to merge 4 commits intomainfrom
Open
feat: 1/7 expose PostgreSQL role parser surface#25521tabVersion wants to merge 4 commits intomainfrom
tabVersion wants to merge 4 commits intomainfrom
Conversation
This starts the RBAC split stack with parser and display support for PostgreSQL role syntax while keeping frontend behavior behind compile-safe unsupported paths. The later stack layers add storage, meta authority, frontend behavior, session semantics, and catalog compatibility.\n\nConstraint: New parser enum variants must not break downstream exhaustive matches before behavior lands.\nRejected: Include full frontend role handling in this parser PR | would collapse the planned review boundary.\nConfidence: high\nScope-risk: narrow\nTested: cargo fmt --check; cargo test -p risingwave_sqlparser granted_by --test sqlparser_common; cargo check -p risingwave_frontend\nNot-tested: Full workspace CI and runtime SQL behavior; covered by later stack PRs
7 tasks
tabVersion
added a commit
that referenced
this pull request
Apr 29, 2026
Avoid treating option-keyword role names as REVOKE <option> OPTION FOR unless OPTION follows, so names like admin, inherit, and set remain valid role targets.\n\nConstraint: PR #25521 only exposes parser surface and should keep the fix localized to sqlparser.\nRejected: reserving admin/inherit/set as role-option prefixes unconditionally | PostgreSQL role names can use these words and the parser already supports identifier role targets.\nConfidence: high\nScope-risk: narrow\nDirective: Keep role-option detection lookahead-based so role names are not consumed before the clause is proven.\nTested: cargo fmt --check; cargo test -p risingwave_sqlparser parse_revoke_role_named_option_words_without_option_clause --test sqlparser_common; cargo test -p risingwave_sqlparser parse_revoke_set_option_for_role --test sqlparser_common; cargo test -p risingwave_sqlparser granted_by --test sqlparser_common\nNot-tested: full workspace check
Avoid treating option-keyword role names as REVOKE <option> OPTION FOR unless OPTION follows, so names like admin, inherit, and set remain valid role targets. Constraint: PR #25521 only exposes parser surface and should keep the fix localized to sqlparser. Rejected: reserving admin/inherit/set as role-option prefixes unconditionally | PostgreSQL role names can use these words and the parser already supports identifier role targets. Confidence: high Scope-risk: narrow Directive: Keep role-option detection lookahead-based so role names are not consumed before the clause is proven. Tested: cargo fmt --check; cargo test -p risingwave_sqlparser parse_revoke_role_named_option_words_without_option_clause --test sqlparser_common; cargo test -p risingwave_sqlparser parse_revoke_set_option_for_role --test sqlparser_common; cargo test -p risingwave_sqlparser granted_by --test sqlparser_common Not-tested: full workspace check
8d17790 to
0d4aceb
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds PostgreSQL-style RBAC SQL surface area (parser + AST + formatting) for role-oriented statements, along with initial frontend/pgwire fallbacks and documentation, as the first lane in the RBAC split stack.
Changes:
- Extend SQL AST/parser to support
CREATE ROLE,ALTER ROLE, role-membershipGRANT/REVOKE(incl.GRANTED BY), plusSET ROLE/RESET ROLE. - Add/extend parser test coverage via
sqlparser_commonunit tests and YAML contract tests (including newrole.yamland updateddrop.yaml). - Add compile-safe handling in pgwire statement-type inference and frontend statement dispatch for new variants (mostly as staged “not implemented” paths).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/pgwire/src/pg_response.rs | Map new role-related Statement variants to existing StatementTypes for response inference. |
| src/sqlparser/tests/testdata/role.yaml | New YAML parser/formatter contract coverage for role syntax, options, and GRANTED BY. |
| src/sqlparser/tests/testdata/drop.yaml | Extend DROP YAML contracts to cover DROP ROLE. |
| src/sqlparser/tests/sqlparser_common.rs | Add unit tests for DROP ROLE, role create/alter, role membership grant/revoke, and set/reset role. |
| src/sqlparser/src/parser.rs | Implement parsing for role statements/options, role-membership grant/revoke, and RESET ROLE. |
| src/sqlparser/src/keywords.rs | Add ADMIN and ROLE keywords. |
| src/sqlparser/src/ast/statement.rs | Add role context/modifier and role-option AST/display support; extend UserOption parsing/display. |
| src/sqlparser/src/ast/mod.rs | Add new Statement variants (GrantRole, RevokeRole, CreateRole, AlterRole, SetRole, ResetRole) and ObjectType::Role display/parse support. |
| src/frontend/src/handler/mod.rs | Add DROP ROLE frontend fallback (explicit not-implemented). |
| src/frontend/src/handler/create_user.rs | Reject newly-parsed role-related user options with a staged “unsupported yet” error. |
| src/frontend/src/handler/alter_user.rs | Reject newly-parsed role-related user options with a staged “unsupported yet” error. |
| docs/dev/src/design/rbac-parser-surface.md | New design note documenting the staged parser/dispatch surface. |
| docs/dev/src/SUMMARY.md | Add the RBAC parser surface doc to the dev docs summary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tabVersion
commented
May 4, 2026
Role membership GRANT/REVOKE was selected only when the first token was not a privilege keyword, so roles named connect, create, select, or usage could be created but not granted or revoked unquoted. The parser now disambiguates by the statement shape: object privileges must reach ON before the role separator, while membership statements reach TO/FROM first.\n\nConstraint: This PR is the parser-surface lane for a stacked RBAC rollout; runtime RBAC semantics remain out of scope.\nRejected: Ban privilege-keyword role names | CREATE ROLE already accepts them and quoted-only follow-up would make the parser contract inconsistent.\nRejected: Full checkpoint fallback parse | broader control-flow change than needed for this grammar split.\nConfidence: high\nScope-risk: narrow\nTested: cargo fmt --check; cargo test -p risingwave_sqlparser --test sqlparser_common; cargo test -p risingwave_sqlparser --test parser_test; manual sqlparser probes for role and object-privilege GRANT/REVOKE\nNot-tested: Full workspace check
Copilot noted that the parser-surface lane documented stale YAML coverage and lacked tests for the staged user-handler boundary around role options. Keep the documentation aligned with the actual role/drop parser fixtures and add focused frontend tests that the newly parsed CREATEROLE/INHERIT variants still fail explicitly until runtime RBAC lanes implement them.\n\nConstraint: This PR should not implement runtime role-option semantics.\nConfidence: high\nScope-risk: narrow\nTested: cargo fmt --check; cargo test -p risingwave_frontend test_create_user_rejects_role_options --lib; cargo test -p risingwave_frontend test_alter_user_rejects_role_options --lib; cargo test -p risingwave_sqlparser --test parser_test -- role.yaml; cargo test -p risingwave_sqlparser --test parser_test -- drop.yaml\nNot-tested: Full ./risedev c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR is 1/7 in the PostgreSQL-style RBAC alignment stack tracked by #25528. It extracts the parser-facing and statement-surface part from the full RBAC alignment work in #25449 so reviewers can validate the SQL grammar, AST, display output, parser tests, and minimal frontend compile glue before the storage/meta/frontend/session/catalog semantics land in later PRs.
User-facing SQL surface added by this PR
Users and downstream RBAC implementation PRs can now rely on RisingWave recognizing and formatting these PostgreSQL-style role-management statements at the parser/AST layer:
CREATE ROLE ...ALTER ROLE ...DROP ROLE ...LOGIN/NOLOGININHERIT/NOINHERITCREATEDB/NOCREATEDBCREATEROLE/NOCREATEROLEGRANT role_name TO member_nameGRANT role_name TO member_name WITH ADMIN OPTIONGRANT role_name TO member_name WITH INHERIT TRUE|FALSE, SET TRUE|FALSEGRANT ... GRANTED BY ...REVOKE role_name FROM member_name [RESTRICT|CASCADE]REVOKE ADMIN|INHERIT|SET OPTION FOR role_name FROM member_nameREVOKE ... GRANTED BY ...SET ROLE role_nameSET ROLE NONESET SESSION ROLE ...SET LOCAL ROLE ...RESET ROLEExample SQL
The intended SQL surface can be understood through examples like:
These examples are meant to document the parser / AST / formatter contract introduced by this PR. The executable RBAC behavior for role storage, authorization checks, session-role effects, and catalog visibility is implemented by the downstream PRs in #25528.
The parser testdata also documents the normalized formatter output for the new statements, so later PRs can implement execution semantics against a stable SQL surface.
Stack / review context
main.Intentional compatibility boundary
This PR is not the full RBAC runtime implementation by itself. It intentionally does not add the authoritative role-membership storage, meta-side grant/revoke/drop behavior, frontend role dispatch/cache behavior,
SET ROLEsession semantics, or PostgreSQL catalog compatibility. Those pieces are split across #25522, #25523, #25524, #25525, #25526, and #25527.Until the full stack lands, the user-facing guarantee of this PR is the parser/AST/display surface and compile-safe frontend boundary for the new statement variants, not complete executable PostgreSQL RBAC behavior.
Tests / verification
cargo fmt --checkcargo test -p risingwave_sqlparser granted_by --test sqlparser_commoncargo check -p risingwave_frontendPart of Tracking RBAC PostgreSQL alignment PR stack #25528
Checklist
Documentation
Release note
This PR is the parser-surface slice of RisingWave's PostgreSQL-style RBAC alignment stack. It makes RisingWave recognize and format PostgreSQL-style role-management SQL such as
CREATE ROLE,ALTER ROLE,DROP ROLE, role-membershipGRANT/REVOKEwith membership options andGRANTED BY, plusSET ROLE/RESET ROLE.The full executable RBAC behavior is intentionally split into follow-up PRs tracked by #25528, so this PR should be read as compatibility groundwork for the SQL surface rather than complete PostgreSQL RBAC runtime support by itself.